EDM-4153: Improve layout of Repository select dropdown#692
Conversation
WalkthroughThis PR removes repository writability validation from the image build wizard and repository selector, and updates repository option rendering plus menu icon alignment styling. ChangesRepository Selection Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Stylelint (17.13.0)libs/ui-components/src/components/form/FormSelect.cssError: ENOENT: no such file or directory, open '/.stylelintrc.json' Comment |
jkilzi
left a comment
jkilzi
left a comment
There was a problem hiding this comment.
Good layout overhaul. The Grid + selectedLabel pattern is a clear improvement over the old Flex-in-description approach.
validateRepoSelection removal: safe to drop — writeAccessOnly: true already filters the list so read-only repos never appear. The client-side double-check was redundant. The handleCreateRepository path is also clean since the create modal enforces write access.
Nit — CSS comment: the .pf-v6-c-menu__item-select-icon absolute-position override is correct but fragile against PatternFly version bumps. Add a short comment explaining the intent, e.g. /* PF v6 checkmark overflows the Grid layout without this — position it absolutely so it doesn't affect column widths */.
Watch — merge ordering: this PR, #698, and #702 all touch RepositorySelect.tsx and/or ConfigWithRepositoryTemplateForm.tsx. Recommended merge order: this PR first (biggest refactor), then #698, then #702 — each subsequent branch just needs a rebase to pick up the prior changes.
celdrake
force-pushed
the
EDM-4153-fix-repo-dropdown-layout
branch
from
f49488c to
2a8c12c
Compare
|
@jkilzi I rebased and fixed the conflicts with previous branches, PTAL when you can. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/ui-components/src/components/form/RepositorySelect.tsx (1)
31-79: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestore writable-only filtering for existing OCI repositories.
Line 41 now filters only by
repo.spec.type, sooptions.writeAccessOnlyno longer excludes previously created OCI repositories whose access mode isREAD.OutputImageStepstill uses this selector for an image push destination, andCreateRepositoryonly forcesREAD_WRITEfor newly created repos, so this change regresses existing read-only repositories from blocked to selectable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/ui-components/src/components/form/RepositorySelect.tsx` around lines 31 - 79, The repository selector in getRepositoryItems no longer respects writable-only filtering, so existing OCI repositories with READ access can still appear selectable. Update the filtering logic used by RepositorySelect to keep excluding read-only OCI repos when options.writeAccessOnly is enabled, while still allowing newly created READ_WRITE repos from CreateRepository; make sure OutputImageStep continues to receive only valid push destinations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@libs/ui-components/src/components/form/RepositorySelect.tsx`:
- Around line 31-79: The repository selector in getRepositoryItems no longer
respects writable-only filtering, so existing OCI repositories with READ access
can still appear selectable. Update the filtering logic used by RepositorySelect
to keep excluding read-only OCI repos when options.writeAccessOnly is enabled,
while still allowing newly created READ_WRITE repos from CreateRepository; make
sure OutputImageStep continues to receive only valid push destinations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 2b4d2e6b-5785-46d6-b55e-2c9bc1b75d65
⛔ Files ignored due to path filters (1)
libs/i18n/locales/en/translation.jsonis excluded by!libs/i18n/locales/en/translation.json
📒 Files selected for processing (3)
libs/ui-components/src/components/ImageBuilds/CreateImageBuildWizard/steps/OutputImageStep.tsxlibs/ui-components/src/components/form/FormSelect.csslibs/ui-components/src/components/form/RepositorySelect.tsx
Made-with: Cursor
celdrake
force-pushed
the
EDM-4153-fix-repo-dropdown-layout
branch
from
2a8c12c to
c4a7104
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/ui-components/src/components/form/RepositorySelect.tsx (1)
31-36: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
writeAccessOnlyis no longer enforced for existing repository choices.
OutputImageStepstill passesoptions={{ writeAccessOnly: true }}, butgetRepositoryItems()now filters only onrepo.spec.typeand never classifies read-only OCI repos as invalid/disabled. After removingvalidateRepoSelection, that means the image build wizard can select an existing read-only registry and fail later on submit/push instead of being blocked here. Please threadwriteAccessOnlyinto the item-building logic and keep non-writable repos out ofvalidRepoItems(or render them disabled ininvalidRepoItems).Also applies to: 45-79, 104-117, 155-166
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/ui-components/src/components/form/RepositorySelect.tsx` around lines 31 - 36, `getRepositoryItems` no longer respects `writeAccessOnly`, so read-only OCI repositories can still appear as selectable existing choices in the image build flow. Update the item-building logic in `RepositorySelect` to thread the `writeAccessOnly` option through `getRepositoryItems` (and any helpers it uses) so repositories with no write access are excluded from `validRepoItems` or moved into `invalidRepoItems` as disabled entries. Make sure the existing `OutputImageStep` usage of `options={{ writeAccessOnly: true }}` continues to prevent selecting non-writable repos.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@libs/ui-components/src/components/form/RepositorySelect.tsx`:
- Around line 31-36: `getRepositoryItems` no longer respects `writeAccessOnly`,
so read-only OCI repositories can still appear as selectable existing choices in
the image build flow. Update the item-building logic in `RepositorySelect` to
thread the `writeAccessOnly` option through `getRepositoryItems` (and any
helpers it uses) so repositories with no write access are excluded from
`validRepoItems` or moved into `invalidRepoItems` as disabled entries. Make sure
the existing `OutputImageStep` usage of `options={{ writeAccessOnly: true }}`
continues to prevent selecting non-writable repos.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 015d4bb1-0514-4e86-a844-699483ae0ff9
⛔ Files ignored due to path filters (1)
libs/i18n/locales/en/translation.jsonis excluded by!libs/i18n/locales/en/translation.json
📒 Files selected for processing (3)
libs/ui-components/src/components/ImageBuilds/CreateImageBuildWizard/steps/OutputImageStep.tsxlibs/ui-components/src/components/form/FormSelect.csslibs/ui-components/src/components/form/RepositorySelect.tsx
2 participants
The layout looks better now, with proper spacing between the selection mark and the additional content.
Removed
validateRepoSelectionas we had made changes to the RepositoryForm to enable only creating valid repositories (for example, only writable OCI registries).Updated shared UI components in
libs/ui-components/to improve the Repository select dropdown layout and simplify repository selection behavior.libs/ui-components/src/components/form/FormSelect.css: adjusted menu item layout and positioned the PatternFly select checkmark absolutely so it no longer overlaps option content.libs/ui-components/src/components/form/RepositorySelect.tsx: removedvalidateRepoSelectionsupport and related validation/error-item generation; repository creation/selection now directly commits the selected/created repo value.libs/ui-components/src/components/ImageBuilds/CreateImageBuildWizard/steps/OutputImageStep.tsx: removed the step’swritableRepoValidationcallback and replaced it with updated helper text indicating only OCI-compliant registries are supported.Cross-cutting impact:
RepositorySelectis a shared component, this affects bothapps/standalone/andapps/ocp-plugin/where the dropdown is used.libs/i18n/locales/en/translation.json: updated strings to align with the new helper text and other copied text changes.eslint.config.js: updated lint rule guidance for PatternFlyModalimports (to use the app’s modal wrapper).No changes were made to the Go auth proxy, container/packaging, CI/workflows, or Cypress/E2E tests.